Skip to content

fix: trim empty top-level else branches#498

Merged
ako merged 1 commit intomendixlabs:mainfrom
hjotha:submit/describer-trim-empty-top-level-else
May 2, 2026
Merged

fix: trim empty top-level else branches#498
ako merged 1 commit intomendixlabs:mainfrom
hjotha:submit/describer-trim-empty-top-level-else

Conversation

@hjotha
Copy link
Copy Markdown
Contributor

@hjotha hjotha commented May 2, 2026

Closes #497.

Summary

  • trims a top-level else if false-branch traversal emits no statements
  • keeps nested IF behavior unchanged, but mirrors its safer tentative-else logic at top level
  • adds a synthetic traversal regression test for a visited shared continuation

Validation

  • go test ./mdl/executor -run 'TopLevelIfRemovesEmptyElse|SequentialIfWithoutElse' -v
  • make build
  • make lint-go
  • make test

Symptom: describe could emit an empty top-level `else` when the false branch pointed at a continuation that had already been emitted while traversing the true branch. Re-executing the MDL removed that empty branch, leaving a roundtrip mismatch.

Root cause: top-level IF traversal decided whether to emit `else` only by comparing `falseFlow.DestinationID` to the selected merge. That misses cases where the false destination is not the merge but traversal still emits no statements because the continuation is already visited.

Fix: mirror nested IF traversal: emit the `else` line tentatively, traverse the false branch, and remove the line again if the traversal produced no statements.

Tests: added a synthetic traversal regression test; `go test ./mdl/executor -run 'TopLevelIfRemovesEmptyElse|SequentialIfWithoutElse' -v`; `make build`; `make lint-go`; `make test`.
@ako
Copy link
Copy Markdown
Collaborator

ako commented May 2, 2026

Review: fix: trim empty top-level else branches

Verdict: Approve

What the PR does

Fixes issue #497: when a false-branch destination was already visited via the true branch, traverseFlowUntilMerge returned immediately at the visited[currentID] guard (cmd_microflows_show_helpers.go:618) without appending anything. The old check falseFlow.DestinationID != mergeID only excluded false branches that pointed directly at the merge — it didn't account for branches pointing at a non-merge node that had already been emitted. The result was an else header with an empty body.

The fix switches to a tentative-emit-then-retract pattern: append the else header, run the traversal, then check if anything was added. If len(*lines) == elseLineIdx+1 (only the else header itself was appended), trim it back.

No blockers

Correctness notes

  • len(*lines) == elseLineIdx+1 is the right sentinel — it fires only when nothing was appended after the else header. If the traversal emits any line (including an inner if ... end if; with no statements), the outer else is correctly kept.
  • The previously-working case (falseFlow.DestinationID == mergeID) still works identically: traverseFlowUntilMerge stops at the merge boundary without appending, then the else is retroactively removed.
  • visitedFalseBranch is a copy of visited at the time of the false branch traversal, so the shared-node scenario the test covers is correctly detected.
  • The PR summary says "at top level" but the change is in traverseFlow which handles all nesting depths — the description is slightly misleading but the logic is correct at all levels.

Positive notes

  • Tentative-emit-then-retract is cleaner than trying to predict traversal output ahead of time — it handles all "empty false branch" causes with one check
  • Test models the exact scenario: outer split → true → inner split → true → shared continuation; outer false branch jumps directly to the same shared node already visited via true
  • Clean PR, no bundled unrelated changes

@ako ako merged commit 3c65b78 into mendixlabs:main May 2, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

describe can emit empty top-level else after shared continuation traversal

3 participants